Bump robrichards/xmlseclibs - #426

phpunit  ./tests/SAML2/SignedElementHelperTest.php --filter=testValidateWithoutModification

export OPENSSL_ENABLE_SHA1_SIGNATURES=1

$this->cryptParams['padding'] = 64; 
// or as Tim discovered 16 works too.

         if( $algo == XMLSecurityKey::RSA_1_5 ) {         
            $key->adjustPadding(16);
        }

public function adjustPadding( $mask )
    {
        $this->cryptParams['padding'] &= ~(RSA::ENCRYPTION_PKCS1);
        $this->cryptParams['padding'] |= $mask;
        return $this;
    }

Ok, I know there were some changes in padding from both the change to phpseclibs as well as in some of the v4 hardening so I just threw the problem at some agents to diagnose. They all basically came up with the same answer (v3 was much more forgiving so didn't enforce proper usage everywhere). It does suggest a change to xmlseclibs to make this more explicit so I am going to at least add that check to the lib.

The bug is in their test fixture, not xmlseclibs. The test signs with an RSA encryption key (XMLSecurityKey::RSA_1_5), so the XML signature literally says SignatureMethod=["http://www.w3.org/2001/04/xmlenc#rsa-1_5"](http://www.w3.org/2001/04/xmlenc#rsa-1_5). v3 only passed by accident.

I reproduced it at their PR head (600dff8) with xmlseclibs 4.0.1 on phpseclib 3.0.57 and 3.0.58. testValidateWithoutModification fails with "Unable to validate Signature", coming from Utils.php:230.

Why it fails

    [CertificatesMock.php:102–117](https://github.com/simplesamlphp/saml2/blob/600dff839440565c69dab6104000c75114a20a82/tests/SAML2/CertificatesMock.php#L102-L117) builds both keys as RSA_1_5. [SignedElementHelperTest.php:33–36](https://github.com/simplesamlphp/saml2/blob/600dff839440565c69dab6104000c75114a20a82/tests/SAML2/SignedElementHelperTest.php#L33-L36) then signs with that private key, and [Utils.php:357–387](https://github.com/simplesamlphp/saml2/blob/600dff839440565c69dab6104000c75114a20a82/src/SAML2/Utils.php#L357-L387) doesn't object.
    In v4, RSA_1_5 sets phpseclib's ENCRYPTION_PKCS1 (2), while real RSA-SHA* types set SIGNATURE_PKCS1 (64) ([XMLSecurityKey.php:230–305](https://github.com/robrichards/xmlseclibs/blob/3d910b23ac1f566c2fa48abf795d710f78a3b770/src/XMLSecurityKey.php#L230-L305)). configureRSAKey() ([:880–907](https://github.com/robrichards/xmlseclibs/blob/3d910b23ac1f566c2fa48abf795d710f78a3b770/src/XMLSecurityKey.php#L880-L907)) passes 2 to withPadding(). That only sets the encryption mode, so the signature mode stays at whatever the loaded key defaulted to.
    Those defaults differ: a bare private key loads with PSS, but a key from an X.509 cert loads with PKCS#1 v1.5 ([phpseclib X509.php:2205–2234](https://github.com/phpseclib/phpseclib/blob/d17e0ddaeaf6f22f7e007cbb437d78792fe2a0e4/phpseclib/File/X509.php#L2205-L2234)). So the test signs with PSS and verifies with PKCS#1 v1.5.

Why "16" makes it pass. In phpseclib 3, 16 is SIGNATURE_PSS ([constants](https://github.com/phpseclib/phpseclib/blob/d17e0ddaeaf6f22f7e007cbb437d78792fe2a0e4/phpseclib/Crypt/RSA.php#L90-L158)). Forcing it in configureRSAKey() just makes verify match the PSS signature. Forcing 64 works too. Neither is a real fix: the signature still declares an encryption URI, and patching configureRSAKey() globally would also change encryption callers.

Why v3 passed. v3.1.5 ([XMLSecurityKey.php:560–594](https://github.com/robrichards/xmlseclibs/blob/5e79a25c52ce219f3b42b9e554de0adb9bfc43bb/src/XMLSecurityKey.php#L560-L594)) called openssl_sign/openssl_verify with their defaults, SHA-1 and PKCS#1 v1.5, no matter what the key type said.



Their fix. Sign with an RSA_SHA256 private key built from the same PEM, verify with their existing getPublicKeySha256(), and keep the RSA_1_5 helpers for encryption tests. I tried that locally as a test-only change, and all 5 tests (13 assertions) pass on unmodified xmlseclibs 4.0.1 with phpseclib 3.0.57.

For xmlseclibs. No padding compatibility change is needed. It would be worth a small hardening change, though: reject encryption-only key types (RSA_1_5, RSA_OAEP*) in signData()/verifySignature() and before DSig writes the SignatureMethod, with tests. Right now v4 doesn't cleanly reject this misuse. It silently produces a signature it can't verify, which is why the error is so confusing for them.

I only ran that one test class, not their full suite. Their checkout is on local branch investigate-426-padding in /workspace/saml2-426, and the fixture patch and logs are in /workspace/saml2-426-results/. Nothing was posted or pushed. I can draft a reply for their PR, or prototype the xmlseclibs rejection on a local branch, if either would help.